Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 187 files, which is 37 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (113)
📒 Files selected for processing (187)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Closes the notifications slice of the org-identity model: a user who
owns/admins a group can see that group's notifications ('your group was
endorsed') alongside their own. Gated behind NOTIFICATIONS_AGGREGATION
(default OFF) because it needs the magic-indexer `recipients` change first
(see docs/org-identity/indexer-notifications-aggregation.md); with the flag
off the page + badge are byte-identical to before.
- config: NOTIFICATIONS_AGGREGATION_ENABLED (NEXT_PUBLIC_, read server +
client).
- notifications client: fetchNotifications/fetchUnreadCount take an
optional `recipients` set (sent only when non-empty) and parse the new
per-notification `recipient` field.
- route: a `recipients` query variant selected ONLY when the flag is on +
recipients are present and valid (DID-shaped, deduped, capped) — so an
indexer without the arg never receives it.
- use-managed-notifications: aggregated feed across useManagedAuthors(),
each row tagged via ownerTagForDid(recipient).
- notifications-context: the unread badge counts across managed recipients
when the flag is on (NotificationsProvider sits under OrgProvider).
- notifications page: an identity focus filter (shared useIdentityFocus)
and a 'via {group}' byline on group rows; group rows are read-only —
the endorsement accept/reject control is suppressed so the viewer can't
respond as their personal account to an award made to the group.
Wrapped in Suspense for the useSearchParams static-prerender bailout.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- managed fixtures: per-recipient notification notices (member group never a recipient) + an aggregated unread count. - mock-fetch-provider: /api/notifications honours `recipients`, serving the managed notifications connection (with `recipient`) + summed unread count. - preview harness: a 'notifications' surface (managed scenario) so the aggregated UI is browser-verifiable with NEXT_PUBLIC_NOTIFICATIONS_AGGREGATION=true. - unit tests: recipients are omitted from the request body by default and included only when non-empty; the `recipient` field round-trips; the fixtures tag recipients correctly and exclude the member group. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… the switcher
Two majors from the review:
- Home sidebar misattribution: switching into a group dropped the 'via
{group}' byline for ALL rows without filtering the list to that group, so
personal + other-group records read as if they belonged to the focused
group. Removed the activeOrg coupling entirely — Home always shows the
full managed aggregate with provenance always visible (acting-as is
read-scope and never strips what's yours); /managed is where the focus
filter narrows to one owner. 'Show more' now points at /managed (the
aggregate it previews).
- Edit eligibility was gated on the org switcher, so a record surfaced via
read-aggregation couldn't be edited without first switching into its
group — breaking the read->edit path. Eligibility now derives from the
managed-author set (personal + owned/admin groups), and both edit pages
wait on the managed-author load for non-personal records so a managed
group record never flashes 'you can't edit'. The BFF still re-checks role.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ViaByline was invisible to screen readers (all children aria-hidden, a
bare aria-label on a roleless span isn't announced). The visible 'via
{name}' text is now the accessible name; the role is appended sr-only.
- PostingAs menu now shows a reserved-width check on the current identity.
- Follow/Endorse pickers stored only the selected DID and derive the
identity each render, so the picker shows the freshest resolved option
instead of a stale postingOptions[0] snapshot.
- Notifications held both feed hooks until the org roles resolve, so a
managing viewer no longer fires a throwaway personal fetch / flashes
personal-only before aggregation engages.
- Focus filters scroll horizontally on narrow viewports (no overflow/wrap);
the notifications dropdown is width-capped to match /managed.
- Profile bridge hover uses --color-accent-hover (theme-correct 'stronger'
in both modes) instead of the lighter --color-accent.
- Dropped an orphan (.managed-row__via-label) and an unused
(.notification-row__via) class.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Extracted the notifications operation allowlist, query variants, recipient validation, and per-op variable normalization into ./operations (flag injected as a param) so the flag-gate is unit-testable without the full route. operations.test.ts asserts the flag-OFF path never emits recipients / never selects the aggregated variant, and the flag-ON path validates + dedups + caps recipients and picks the variant. parseRecipients now reuses isValidDid instead of a looser did:-prefix check. - owner-tag.test.ts pins the tagging contract incl. the 'never label a stranger You' safety branch. - fan-out.test.ts pins per-DID error isolation + AbortError re-throw. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tion feat: org-identity aggregation — surface group-owned work, per-action write identity, flag-gated notifications
…r-action write identity, flag-gated notifications"
…ity-aggregation Revert "feat: org-identity aggregation — surface group-owned work, per-action write identity, flag-gated notifications"
Shared primitives for showing records owned by the groups a user
owns/admins on their own surfaces, tagged 'by {group}':
- lib/groups/managed.ts: ownedOrAdminGroups (member-role excluded) +
managedAuthorDids (the 'My X' author set — viewer + owned/admin groups,
or just the active group when acting-as), both pure + tested.
- use-managed-authors / use-managed-projects / use-managed-activities:
aggregate fetchProjects/fetchIndexerActivities across the managed author
set and owner-tag each record; the project/activity hooks take an
`enabled` flag so a caller can switch aggregation off (foreign profiles)
without breaking the rules of hooks.
- atproto/owner-tag.ts: ownerTagForDid/ownerTagForUri — never mislabels a
stranger's record as 'You' (tested).
- ui/owner-byline.tsx: the compact 'by {group}' byline (avatar + name; role
announced sr-only).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
'My projects' / 'My activities' now read through the managed hooks, so
records owned by the groups the viewer owns/admins surface alongside their
personal ones, each tagged 'by {group}'. Personal records carry no byline.
The sidebar is personal-anchored (acting-as is read-scope and never changes
what's 'yours' here).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ions' - 'My projects' / 'My activities' (the by-me filter) now query the managed author set (viewer + owned/admin groups), so group-owned records appear, attributed to the group by the row's existing author column. - 'My organizations' filter fixed: it resolved groups by scanning the most-recently-indexed top-100 actors and filtering client-side, so any org outside that window silently dropped. It now resolves the viewer's group DIDs DIRECTLY via fetchNetworkActorsByDids — every org shows. The stale top-100 client-filter clause + its now-wrong comment are removed so the path can't be reintroduced. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On the viewer's OWN personal profile only (isOwnProfile && !activeOrg) the
tabs aggregate records owned by the groups the viewer owns/admins; foreign
and group profiles are unchanged.
- Projects tab: aggregated boxes render a 'by {group}' byline under the
title; the aggregated set can span pages, so the own-profile view
paginates (Load more).
- Activities tab: useUserIndexerActivities gains an optional authoredAuthors
set — the 'Created' bucket fans out across the managed authors via the
multi-author op, so group-authored activities appear with the group shown
as the card's author (the dids map carries the owning DID). 'Contributed'
stays the viewer's personal contributions. A generation guard drops a
stale page if the author set changes mid-loadMore.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The auth-mock preview harness gains a ?managed=1 scenario: the mock session
belongs to three groups (owner/admin/member) and the indexer serves the
records authored by the owner/admin ones, so the inline 'by {group}'
aggregation on the Home sidebar + the profile Projects/Activities tabs can
be browser-verified logged-out. Member-role exclusion is pinned at the
fixture level. Dev-only — the preview route notFound()s in production.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Navigation: - Top bar: add Home, Explore, and My profile icons around the existing Apps/Settings cluster (auth-gated where appropriate). - Left rail: move Apps to directly after Explore. - Create flows (/create, /project/new) now get the row-2 Back affordance. Explore: - List-row author bylines: zero the card-view bottom margin inside .cert-list-row__author-col so they sit vertically centered. - "My organizations" filter now renders every group the viewer belongs to (synthesised per-row), not just those the indexer has a profile for. Edit project form: - EditBanner Save button rendered dark-on-dark: two equal-specificity arbitrary Tailwind text utilities resolved by stylesheet order. Force the primary label color with !text so it always wins. - Constrain the editing banner to the article's reading column (.edit-banner--project) instead of the full --wide content width. Permissions: - Show the Edit button on an activity/project when the viewer is an owner/admin of the owning group but signed in as their individual user. Clicking opens a confirm modal explaining they'll edit as the group, then switches into it and opens the editor. Delete stays gated to direct owners. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The /search people-search page is redundant with /explore. Remove the route and repoint every "Explore" nav entry (left rail, mobile sidebar, bottom nav) at /explore. Add a permanent /search -> /explore redirect so old / indexed links don't 404, drop /search from robots disallow, and delete the now-orphaned PeopleSearch component. The global search field and /api/search-actors typeahead are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…n edge The banner was capped to the 960px column but the article column has 16px side padding, so the hero/title sit in a 928px box and the banner stuck out 16px each side. Cap the banner to the column width minus that padding so its edges line up with the visible content. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The home-feed row's date track was auto-sized, so a relative "2d ago" and an absolute "2026-05-20" produced different column widths and the dates didn't align across rows. Pin the date track to 84px (matching the explore list) and right-align so every date sits in the same box. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stack the icon + label vertically for Home, Explore, Apps, Profile and Settings in the desktop top bar, LinkedIn-style. Drop the now- redundant title tooltips; keep aria-labels for screen readers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Headline feature: Phase 2 endorsement response flow
The recipient now has an explicit lever to hide unwanted endorsements via
app.certified.badge.response. Closes the structural gap left by Phase 1: anyone could endorse anyone, but the recipient had no curation surface. Default-show stays the default (low friction); an opt-out (Hide) writes arejectedresponse on the recipient's own PDS and filters the award out of profile read paths.What ships
/notificationsrows where the underlying record is abadge.award. Loud, inline./endorsementsReceived tab. State indicator inside the menu ("Showing on your profile" / "Hidden from your profile" / etc.) plus actions for the current state.badge.award.aria-live="polite", single-click revert).Architecture decisions
Captured in writing per deep-flow §"Decisions belong in writing":
docs/badge-response-flow/plan.md— alternatives considered for default visibility (opt-out chosen), action-surface placement (loud notifications + quiet kebab), and the write strategy (append-only with rkey lexicographic tie-break for createdAt collisions).docs/badge-response-flow/review-round-1.md— three parallel reviewer agents on the plan, each finding consolidated as accept/reject with rationale. Items folded back into the plan in place.docs/badge-response-flow/review-round-1-impl.md— three parallel reviewer agents on the implementation, six critical items addressed in a follow-up commit (cross-hook staleness viauseSyncExternalStore, focus + undo toast, roving tabindex, aria-label disambiguation, mobile touch targets, owner-only state indicator).Accumulated non-feature work
Same PR, separate set of commits:
docs(agents)— adds the deep-flow process spec to AGENTS.md §26.feat(endorsements)— Phase 1 migration ontoapp.certified.badge.{definition,award}from the legacyapp.certified.temp.graph.endorsementlexicon (each user owns their endorsement badge).fix(cache),fix(profile),fix(edit-profile),fix(left-rail),fix(news),fix(about),fix(legal),fix(apps),feat(news),refactor(groups)— accumulated polish since main was last cut. Per-commit messages describe scope.Breaking changes
None at the contract level. The endorsement lexicon migration (Phase 1) deprecates
app.certified.temp.graph.endorsementfor new writes but the proxy allowlist still accepts the legacy collection so the feed-side "trusted evaluator" filter keeps working. A separate follow-up will migrate the feed filter onto badges.Out of scope (filed as follow-ups)
badge.awardrate-limiting on the xrpc proxy — abuse-mitigation, deferred per plan §"Out of scope" D1. Track as a separate security PR.badge.award— backend extension that letsapp.certified.badge.awardfirehose events emit "endorsement" notifications. Today only legacy temp lexicon notifications surface. The nav-badge counter mitigates the discovery gap; the proper fix lives in the notifications backend (separate issue against the indexer team).hb-agent/magic-indexer#65. When that lands, the PDS fan-out scan inuseReceivedEndorsementscollapses to a single GraphQL query.hb-agent/magic-indexer#67(PR landed forapp/certified/*) andhb-agent/magic-indexer#68(heads-up fororg/hypercerts/*).Verification
npx tsc --noEmit— 0 errorsnpx eslint src/— 0 errors, 28 warnings (baseline 27; +1 from a latest-ref-in-useEffect pattern inuse-profile-responses; within plan AC#10 ±2 budget)npx next build— green/endorsements"+ New"/endorsementsReceived tab — see A's endorsement🤖 Generated with Claude Code